Skip to content

API: remove SigningResult #862

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
tetsuo-cpp merged 12 commits into from
Jan 15, 2024
Merged

API: remove SigningResult #862

tetsuo-cpp merged 12 commits into from
Jan 15, 2024

Conversation

@woodruffw
Copy link
Member

@woodruffw woodruffw commented Jan 12, 2024

This removes the SigningResult model, and replaces it with Bundle in APIs where a SigningResult was previously returned.

Closes #854.

Unblocks #860.

cc @laurentsimon for visibility 🙂

@woodruffw
remove SigningResult
36cb2bb 
Signed-off-by: William Woodruff <william@trailofbits.com>
@woodruffw woodruffw added component:signing Core signing functionality component:api Public APIs labels Jan 12, 2024
@woodruffw woodruffw requested review from jku and tetsuo-cpp January 12, 2024 16:21
@woodruffw woodruffw self-assigned this Jan 12, 2024
@woodruffw
CHANGELOG: links
06f12a6 
Signed-off-by: William Woodruff <william@trailofbits.com>
@woodruffw
test: annotations
417c16f 
Signed-off-by: William Woodruff <william@trailofbits.com>
@woodruffw
test_sign: begin fixing tests
1fef8cc 
Signed-off-by: William Woodruff <william@trailofbits.com>
@woodruffw
more test fixes
ae31928 
Signed-off-by: William Woodruff <william@trailofbits.com>
@woodruffw
test: b64
011a5da 
Signed-off-by: William Woodruff <william@trailofbits.com>
@woodruffw
test: more munging
57d6630 
Signed-off-by: William Woodruff <william@trailofbits.com>
@woodruffw
sigstore: fixup sign
97cf8ae 
Signed-off-by: William Woodruff <william@trailofbits.com>
github-advanced-security[bot]
github-advanced-security bot found potential problems Jan 12, 2024
View reviewed changes
sigstore/_cli.py
if outputs["cert"] is not None:
with outputs["cert"].open(mode="w") as io:
print(result.cert_pem, file=io)
print(cert_pem, file=io)

Check failure

Code scanning / CodeQL

Clear-text logging of sensitive information

This expression logs [sensitive data (certificate)](1) as clear text. This expression logs [sensitive data (certificate)](1) as clear text.
@woodruffw
Copy link
Member Author

woodruffw commented Jan 12, 2024

This should be good to go -- the Bundle attributes are slightly different than the old SigningResult ones, but just about everything relevant is still present (the log entry UUIDs are gone, but these were only part of the Rekor REST API anyways).

@woodruffw woodruffw added this to the 3.0 milestone Jan 12, 2024
@woodruffw
Copy link
Member Author

woodruffw commented Jan 12, 2024

(More generally, this makes me wonder if we can remove LogEntry too and just use the protobuf-specs TransparencyLogEntry. But that might be harder of a lift.)

@laurentsimon laurentsimon mentioned this pull request Jan 12, 2024
Merged
tetsuo-cpp
tetsuo-cpp approved these changes Jan 14, 2024
View reviewed changes
Copy link
Contributor

@tetsuo-cpp tetsuo-cpp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@tetsuo-cpp tetsuo-cpp merged commit 5d53e26 into main Jan 15, 2024
@tetsuo-cpp tetsuo-cpp deleted the ww/rm-signingresult branch January 15, 2024 02:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jku jku Awaiting requested review from jku

1 more reviewer

@tetsuo-cpp tetsuo-cpp tetsuo-cpp approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@woodruffw woodruffw

Labels

component:api Public APIs component:signing Core signing functionality

Projects

None yet

Milestone

3.0

Development

Successfully merging this pull request may close these issues.

API: Remove SigningResult

3 participants

@woodruffw @tetsuo-cpp